Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a custom GyrDiskService for ActiveStorage to workaround CircleCI issues #1885

Conversation

tgrathwell
Copy link
Contributor

As seen in this issue, certain versions of Docker and certain versions of the Linux
kernel manifest a problem where creating tempfiles wiht IO.copy_stream doesn't work:
docker/for-linux#1015

CircleCI seems to have upgraded their linux kernels and now we're seeing this problem.
A temporary workaround is to change the ActiveStorage code to not use IO.copy_stream
by making a custom service (since the Disk service is only for test anyway so we can
do whatever we want)

@tgrathwell tgrathwell temporarily deployed to vita-min-custom-actives-t4ggbe December 3, 2021 20:58 Inactive
@tgrathwell tgrathwell force-pushed the custom-activestorage-service-to-make-circleci-work-for-now-#180524323 branch from 0d8f70e to 3519117 Compare December 3, 2021 20:58
@tgrathwell tgrathwell temporarily deployed to vita-min-custom-actives-t4ggbe December 3, 2021 20:59 Inactive
@tgrathwell tgrathwell force-pushed the custom-activestorage-service-to-make-circleci-work-for-now-#180524323 branch from 3519117 to cecf7cf Compare December 3, 2021 21:06
@tgrathwell tgrathwell temporarily deployed to vita-min-custom-actives-t4ggbe December 3, 2021 21:07 Inactive
Copy link
Contributor

@caitbot caitbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good to me and makes sense as a work-around

# We should be able to remove this if CircleCI starts using a different linux kernel version (currently 5.4.0-1060-aws)

File.write(make_path_for(key), io.read)
io.rewind
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do rewind? is this because it is the state that IO.copy_stream leaves io in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some paranoia about IO files getting angry if someone tries to read them twice, though I didn't actually see that problem in real life yet for this code

everything I've tried so far has made some tests pass but others fail; I've done another revision where it only tries to File.write if the io object is an actual file, let's see if that fares any better

…cleCI issues

As seen in this issue, certain versions of Docker and certain versions of the Linux
kernel manifest a problem where creating tempfiles wiht `IO.copy_stream` doesn't work:
docker/for-linux#1015

CircleCI seems to have upgraded their linux kernels and now we're seeing this problem.
A temporary workaround is to change the ActiveStorage code to not use IO.copy_stream
by making a custom service (since the Disk service is only for test anyway so we can
do whatever we want)
@tgrathwell tgrathwell force-pushed the custom-activestorage-service-to-make-circleci-work-for-now-#180524323 branch from cecf7cf to 506af86 Compare December 3, 2021 21:14
@tgrathwell tgrathwell temporarily deployed to vita-min-custom-actives-t4ggbe December 3, 2021 21:14 Inactive
@tgrathwell tgrathwell merged commit 40b07a9 into main Dec 3, 2021
@tgrathwell tgrathwell deleted the custom-activestorage-service-to-make-circleci-work-for-now-#180524323 branch December 3, 2021 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants